Skip to content

docs(providers): align setup guidance - #434

Open
deepujain wants to merge 2 commits into
NVIDIA:mainfrom
deepujain:docs/433-provider-setup
Open

docs(providers): align setup guidance#434
deepujain wants to merge 2 commits into
NVIDIA:mainfrom
deepujain:docs/433-provider-setup

Conversation

@deepujain

@deepujain deepujain commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Align .env.example, the README provider reference, and the development setup guide with the provider selector on current main.
  • Document the dedicated Ollama, Azure OpenAI, generic OpenAI-compatible, and Gemini CLI setup paths.
  • Preserve existing provider behavior; this PR changes setup guidance only.

Validation

  • Compared every documented provider name, credential variable, endpoint default, and model default against the corresponding implementation under src/skillspector/providers/.
  • Verified all eleven usable provider names and the new provider-specific variables occur in .env.example, README.md, and docs/DEVELOPMENT.md.
  • pytest -m 'not integration and not provider' tests/ -q - 2,825 passed, 13 skipped, 38 deselected, 4 xfailed.
  • ruff check src tests - passed.
  • ruff format --check src tests - 193 files already formatted.
  • git diff --check - passed.
  • Inspected the Markdown tables and fenced setup examples in source; no local Markdown renderer is configured in the repository.

Risk

  • Low: documentation and the example environment template only; runtime provider selection and request behavior are unchanged.
  • Live-provider tests were not run because no provider code changed.

Fixes #433

@deepujain
deepujain force-pushed the docs/433-provider-setup branch from 8cf2c56 to b62550f Compare August 25, 2026 22:11
rng1995
rng1995 requested changes Aug 28, 2026
View reviewed changes

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

The added provider setup paths otherwise match their implementations, but the table would still advertise nv_build's retired default after merging into current main. Please update that model reference so the setup documentation is actually aligned. All required checks are green.

Comment thread README.md Outdated
@deepujain

deepujain commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Updated the nv_build row to the current z-ai/glm-5.2 default requested in review. The latest CI run is green; ready for re-review.

@deepujain
deepujain force-pushed the docs/433-provider-setup branch 2 times, most recently from 2ce0642 to c053700 Compare September 1, 2026 17:57
@deepujain

deepujain commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Re-review ping: nv_build default is z-ai/glm-5.2 on current head c053700; all checks green.

@deepujain

deepujain commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Re-review request on head c0537009:

The Aug 28 nv_build default comment is addressed: provider setup docs now reference z-ai/glm-5.2 to match current NvBuildProvider.DEFAULT_MODEL on main.

CI green on this head. Prior review thread is outdated. Please re-review when convenient.

@deepujain
deepujain force-pushed the docs/433-provider-setup branch 3 times, most recently from 1150743 to 16c769b Compare September 9, 2026 21:54
rng1995
rng1995 approved these changes Sep 12, 2026
View reviewed changes

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Reviewed current head 16c769b3819e46869e8abba00ed1acd2fdaf8699. The prior documentation issue is resolved: the nv_build provider row now uses the current z-ai/glm-5.2 default, and the remaining provider names, credentials, endpoints, and defaults align with the implementations. I found no remaining required change.

All hosted required checks pass and the prior thread is resolved. The branch is behind main, so it must be updated and revalidated before merging.

@deepujain
docs(providers): align setup guidance
a090594 
Signed-off-by: Deepak Jain <deepujain@gmail.com>
@deepujain
docs(providers): update the nv_build default model
01e28d2 
Signed-off-by: Deepak Jain <deepujain@gmail.com>
@deepujain
deepujain force-pushed the docs/433-provider-setup branch from 16c769b to 01e28d2 Compare September 12, 2026 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rng1995 rng1995 rng1995 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Provider setup docs omit supported backends

2 participants

@deepujain @rng1995